Scal xyb by aronroland · Pull Request #556 · NOAA-EMC/WW3

aronroland · 2021-12-07T18:22:56Z

Pull Request Summary

A short overview of the PR
Remove XYB and replaced by double precision XGRD, YGRD and single precision ZB
https://github.com/NOAA-EMC/WW3/projects/2

Description

XYB is keeping the XY-coordinates and the depths, this array is deleted for the sake of memory saving and towards the integration of the scalability branch into develop. We rather use XGRD, YGRD and ZB instead. XGRD and YGRD are converted to double precision since this is needed by the unstructured part of WW3 for the accurate calculation of the geometric quantities of the unstructured triangular grid. The changes have been implemented in such a way that the structured part of the code and the non-decomposed part of the unstructured grid remains constant. The use of single precision in lat/lon coordinates may also be a bug for the structured, curvilinear, triangular or smc grid if applied to very high resolution.

Please also include the following information:

Add any suggestions for a reviewer
enhancement
Are answer changes expected from this PR? Please describe the changes and the reason why in addition to which of the mod_def change only for the unstructured part

Issue(s) addressed

Please list any issues associated with this PR, including those the PR will fix/close. For example:

fixes double precision is needed for high resolution applications #557

Commit Message

Remove XYB and replaced by double precision XGRD, YGRD and single precision ZB

Check list

Branch is up to date with the authoritative repository (NOAA-EMC) develop branch.
Checked the checklist for a developer submitting to develop.
If a version number update is required, checked the updating version number checklist.
If a new feature was added, a regression test for testing the new feature is added.

Testing

How were these changes tested? Regression test on NOAA HPC
Are the changes covered by regression tests? Yes
Have the matrix regression tests been run? Yes, Intel on NOAA HPC

These two tests are not expected. Aron has them identical with Intel and Ali has them different on both Orion and Hera using Intel. @thesser1 could you test this test on ERDC HPC or your computer? One does not need mpi.

ww3_tr1/./work                     (1 files differ)
ww3_tr1/./work_std                     (1 files differ)

pre-known different cases:

**********************************************************************
********************* non-identical cases ****************************
**********************************************************************
mww3_test_03/./work_PR2_UQ_MPI_d2                     (6 files differ)
mww3_test_03/./work_PR2_UNO_MPI_d2                     (6 files differ)
mww3_test_03/./work_PR3_UQ_MPI_d2_c                     (8 files differ)
mww3_test_03/./work_PR1_MPI_d2                     (12 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2_c                     (6 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2                     (8 files differ)
mww3_test_03/./work_PR3_UQ_MPI_d2                     (8 files differ)
mww3_test_07/./work_PR3_UQ                     (4 files differ)
ww3_tp2.10/./work_MPI_OMPH                     (6 files differ)
ww3_tp2.16/./work_MPI_OMPH                     (4 files differ)
ww3_ufs1.3/./work_a                     (2 files differ)

Expected changes in mod_def files for unstructured cases:

ww3_tp2.17/./work_b                     (1 files differ)
ww3_tp2.17/./work_mb                     (1 files differ)
ww3_tp2.17/./work_mc                     (1 files differ)
ww3_tp2.17/./work_a                     (1 files differ)
ww3_tp2.17/./work_ma1                     (1 files differ)
ww3_tp2.17/./work_c                     (1 files differ)
ww3_tp2.17/./work_ma                     (1 files differ)
ww3_tp2.17/./work_mc1                     (1 files differ)
ww3_tp2.18/./work_TIDE_MPI                     (1 files differ)
ww3_tp2.6/./work_ST4                     (1 files differ)
ww3_tp2.6/./work_pdlib                     (1 files differ)
ww3_tp2.6/./work_ST0                     (1 files differ)
ww3_tp2.7/./work_ST0                     (1 files differ)
ww3_ts4/./work_ug_MPI                     (1 files differ)

Please indicate the expected changes in the regression test output (Note the known list of non-identical tests).
Please provide the summary output of matrix.comp (matrix.Diff.txt, matrixCompFull.txt and matrixCompSummary.txt):

matrixCompFull.txt
matrixCompSummary.txt
matrixDiff.txt

)

…following the quest

JessicaMeixner-NOAA · 2021-12-07T18:36:49Z

@aronroland Thanks for these nice updates - I can definitely see how this could impact any high-resolution case. I'm curious if you or @aliabdolali looked into the impact of this change (beyond not changing answers) for structured multi-grid test cases that are already memory intensive, for example, the fully resolution test case example in ww3_ufs1.1. Does this dramatically affect the memory usage for that test? This is important information as double versus single precision is not an option in the way this change is introduced.

JessicaMeixner-NOAA

Is there a reason that build_utils.sh and ft2src.sh were changed? This seems unrelated to this PR along with the additional comp/link files.

JessicaMeixner-NOAA · 2021-12-07T18:48:24Z

      DEALLOCATE(MX1, MXX, MYY, MXY, MAPOUT)
      DEALLOCATE(MX1R, MXXR, MYYR, MXYR)
      DEALLOCATE(AUX1)
+!AR: This needs to be cleaned for the scalability part ..


Clean up comment?

JessicaMeixner-NOAA · 2021-12-14T14:52:57Z

@aliabdolali @aronroland there are still a few unresolved issues with this PR from the comments I made in the review last week. There are also the open question as to why the permissions are being changed on a few files. Lastly, there is the question on the impact of the memory since this is not added as an option. Please let me know if I need to look into that myself for impacts to the operations of the global systems here at NCEP.

aliabdolali · 2021-12-15T14:53:13Z

@aliabdolali @aronroland there are still a few unresolved issues with this PR from the comments I made in the review last week. There are also the open question as to why the permissions are being changed on a few files. Lastly, there is the question on the impact of the memory since this is not added as an option. Please let me know if I need to look into that myself for impacts to the operations of the global systems here at NCEP.

@JessicaMeixner-NOAA the way @aronroland and I isolated and tested the scal_xyb does not affect the runtime and performance/outputs of the structured part of the code. As a sanity check, I checked the ufs1.* cases and there is no diff in the runtime. The reason we isolated the work is even one single change in the unstructured case made some changes in the structured part which sometimes takes hours to track and understand, sometimes impossible, therefore delaying the scalability reintegration and further development.
Aron can comment more here.

JessicaMeixner-NOAA · 2021-12-15T14:55:41Z

@aliabdolali it's great to hear runtime is unaffected. How about the memory usage in the ufs1.1 with the high resolution grids? Again, if I need to do this work, I can I just need to know.

aronroland · 2021-12-15T15:10:24Z

@JessicaMeixner-NOAA <https://github.com/JessicaMeixner-NOAA> , @aliabdolali <https://github.com/aliabdolali> , Sorry for the late reply. I was busy this week and will have just time at the weekend to clean the code and reply to the comments from <https://github.com/JessicaMeixner-NOAA> @JessicaMeixner-NOAA, many thanks for this in advance. Since we have deleted one array I do not expect big surprise. How do you <https://github.com/JessicaMeixner-NOAA> @JessicaMeixner-NOAA check memory usage? Cheers Aron Von: Ali.Abdolali ***@***.***> Gesendet: Mittwoch, 15. Dezember 2021 15:53 An: NOAA-EMC/WW3 ***@***.***> Cc: Aron Roland ***@***.***>; Mention ***@***.***> Betreff: Re: [NOAA-EMC/WW3] Scal xyb (PR #556) @aliabdolali <https://github.com/aliabdolali> @aronroland <https://github.com/aronroland> there are still a few unresolved issues with this PR from the comments I made in the review last week. There are also the open question as to why the permissions are being changed on a few files. Lastly, there is the question on the impact of the memory since this is not added as an option. Please let me know if I need to look into that myself for impacts to the operations of the global systems here at NCEP. @JessicaMeixner-NOAA <https://github.com/JessicaMeixner-NOAA> the way @aronroland <https://github.com/aronroland> and I isolated and tested the scal_xyb does not affect the runtime and performance/outputs of the structured part of the code. As a sanity check, I checked the ufs1.* cases and there is no diff in the runtime. The reason we isolated the work is even one single change in the unstructured case made some changes in the structured part which sometimes takes hours to track and understand, sometimes impossible, therefore delaying the scalability reintegration and further development. Aron can comment more here. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#556 (comment)> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB2S43W2YESWUGR3SEQW5ADURCTWJANCNFSM5JR5BF4Q> . <https://github.com/notifications/beacon/AB2S43TZHERACYI75HXB4S3URCTWJA5CNFSM5JR5BF42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOHNGHSVY.gif>

JessicaMeixner-NOAA · 2021-12-15T15:16:47Z

@aronroland there are a variety of ways to check the memory, including using the MEMCHECK switch you implemented. Some HPCs give memory output and we can also check the resulting size of files. Looks like I should just do these checks myself at this point. I hope to have information for you in a week if not before.

aronroland · 2021-12-15T16:32:24Z

Hi Jessica, ok, so I guess that Ali and me can do that as well, as he is back. I would appreciate if could have a discussion with you and Ali in order to organize further steps. Cheers Aron Von: Jessica Meixner ***@***.***> Gesendet: Mittwoch, 15. Dezember 2021 16:17 An: NOAA-EMC/WW3 ***@***.***> Cc: Aron Roland ***@***.***>; Mention ***@***.***> Betreff: Re: [NOAA-EMC/WW3] Scal xyb (PR #556) @aronroland <https://github.com/aronroland> there are a variety of ways to check the memory, including using the MEMCHECK switch you implemented. Some HPCs give memory output and we can also check the resulting size of files. Looks like I should just do these checks myself at this point. I hope to have information for you in a week if not before. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#556 (comment)> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB2S43TDTKALZIASWT4KPQ3URCWOVANCNFSM5JR5BF4Q> . <https://github.com/notifications/beacon/AB2S43UBLYMIIYU2CMOTMODURCWOVA5CNFSM5JR5BF42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOHNGNLRI.gif>

JessicaMeixner-NOAA · 2021-12-16T22:32:00Z

For the ww3_tr1 files that are different, it's the mod_def files. I wonder if something is written to the mod_def files that are changed by the changes made here or alternatively if this is a case of the memory issues that we seem to occasionally get. One question would be if we run this branch twice, does that test give different answers?

aronroland · 2021-12-17T05:02:02Z

Dear Jessica, this is a bit strange stuff. So basically the mod_def file is different for Ali not for me and I cannot reproduce the difference, for which the final run results are the same. As we have looked at Ali’s binary difference using vdiffbin, the results have very tiny difference. I would have fixed it but I cannot reproduce it since using the intel compiler in our system (ifort version 2021.2.0), I could not find any difference in the binary. Code wise there should not be any difference but u never know. So what we wanted to do is to push this to Ty and see what he gets. I can also check on the French super computer. Hunting the differences down for the mod_def, would basically now result for me to introduce some possibility to write everything out in ascii and see where it differs. We will discuss this issue more on Monday I guess. Cheers Aron Von: Jessica Meixner ***@***.***> Gesendet: Donnerstag, 16. Dezember 2021 23:32 An: NOAA-EMC/WW3 ***@***.***> Cc: Aron Roland ***@***.***>; Mention ***@***.***> Betreff: Re: [NOAA-EMC/WW3] Scal xyb (PR #556) For the ww3_tr1 files that are different, it's the mod_def files. I wonder if something is written to the mod_def files that are changed by the changes made here or alternatively if this is a case of the memory issues that we seem to occasionally get. One question would be if we run this branch twice, does that test give different answers? — Reply to this email directly, view it on GitHub <#556 (comment)> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB2S43VCEN42ZEDPAHJE5Y3URJSGVANCNFSM5JR5BF4Q> . You are receiving this because you were mentioned. <https://github.com/notifications/beacon/AB2S43QJEMVKINFXRND2ADLURJSGVA5CNFSM5JR5BF42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOHNQXYPY.gif> Message ID: ***@***.*** ***@***.***> >

ukmo-ccbunney · 2021-12-17T10:17:16Z

Hi @JessicaMeixner-NOAA and @aronroland
Regarding he differences you are seeing in the mod_def.ww3 file for ww3_tr1, I have encountered this before and tracked it down to an uninitialized variable. See #425

Seeing this jogged my memory - I thought a fix had been merged for that Issue, but I don't believe it has.
Perhaps you could test whether it fixes your problem (the details of the fix are in the issue) and I will raise a PR for it.

Cheers.

aronroland · 2021-12-17T10:58:59Z

This is great that u have reported that. Many thanks for this!

JessicaMeixner-NOAA · 2021-12-17T16:14:50Z

The size of the output files did not change, additionally the total memory usage on all of the ww3_ufs* tests increase with this PR, but not so much so that I think this warrants needing to put in an option for this variable. If anyone is concerned about total memory usage for their applications please test this PR and make sure it's okay (@mickaelaccensi @ukmo-ccbunney & anyone else).

aronroland · 2021-12-18T04:50:25Z

Hi Jessica, can u please specify how u have tested the memory usage and post the certain numbers u have obtained? Cheers Aron Von: Jessica Meixner ***@***.***> Gesendet: Freitag, 17. Dezember 2021 17:15 An: NOAA-EMC/WW3 ***@***.***> Cc: Aron Roland ***@***.***>; Mention ***@***.***> Betreff: Re: [NOAA-EMC/WW3] Scal xyb (PR #556) The size of the output files did not change, additionally the total memory usage on all of the ww3_ufs* tests increase with this PR, but not so much so that I think this warrants needing to put in an option for this variable. If anyone is concerned about total memory usage for their applications please test this PR and make sure it's okay ***@***.*** <https://github.com/mickaelaccensi> @ukmo-ccbunney <https://github.com/ukmo-ccbunney> & anyone else). — Reply to this email directly, view it on GitHub <#556 (comment)> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB2S43RHJZAB6LBSHZYDDX3URNOYLANCNFSM5JR5BF4Q> . You are receiving this because you were mentioned. <https://github.com/notifications/beacon/AB2S43RPX4VZRSSPQ2MV7EDURNOYLA5CNFSM5JR5BF42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOHNVKMBQ.gif> Message ID: ***@***.*** ***@***.***> >

JessicaMeixner-NOAA · 2021-12-18T12:45:56Z

@aronroland I looked at the resulting file sizes and I used slurm commands to see the total memory usage of the regression test jobs for the ww3_ufs* tests. The increase in total memory used was about .1GB or less. I didn't save output.

aliabdolali · 2021-12-21T13:44:55Z

For the ww3_tr1 files that are different, it's the mod_def files. I wonder if something is written to the mod_def files that are changed by the changes made here or alternatively if this is a case of the memory issues that we seem to occasionally get. One question would be if we run this branch twice, does that test give different answers?

I checked it this morning, running tr1 twice ended with a different mod_def :(

aliabdolali · 2021-12-24T12:40:50Z

After ww3_tr1 fix and resolving comments, I reran the regtests and here is the summary:

pre-known different cases:

**********************************************************************
********************* non-identical cases ****************************
**********************************************************************
mww3_test_03/./work_PR2_UQ_MPI_d2                     (8 files differ)
mww3_test_03/./work_PR2_UNO_MPI_d2                     (6 files differ)
mww3_test_03/./work_PR3_UQ_MPI_d2_c                     (8 files differ)
mww3_test_03/./work_PR1_MPI_d2                     (10 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2_c                     (8 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2                     (8 files differ)
mww3_test_03/./work_PR3_UQ_MPI_d2                     (8 files differ)
mww3_test_07/./work_PR3_UQ                     (1 files differ)
ww3_tp2.10/./work_MPI_OMPH                     (7 files differ)
ww3_tp2.16/./work_MPI_OMPH                     (5 files differ)
ww3_ufs1.3/./work_a                     (1 files differ)

Expected changes in mod_def files for unstructured cases:

ww3_tp2.17/./work_b                     (1 files differ)
ww3_tp2.17/./work_mb                     (1 files differ)
ww3_tp2.17/./work_mc                     (1 files differ)
ww3_tp2.17/./work_a                     (1 files differ)
ww3_tp2.17/./work_ma1                     (1 files differ)
ww3_tp2.17/./work_c                     (1 files differ)
ww3_tp2.17/./work_ma                     (1 files differ)
ww3_tp2.17/./work_mc1                     (1 files differ)
ww3_tp2.21/./work_b                     (1 files differ)
ww3_tp2.21/./work_mb                     (1 files differ)
ww3_tp2.21/./work_a                     (1 files differ)
ww3_tp2.21/./work_ma                     (1 files differ)
ww3_tp2.6/./work_ST4                     (1 files differ)
ww3_tp2.6/./work_pdlib                     (1 files differ)
ww3_tp2.6/./work_ST0                     (1 files differ)
ww3_tp2.7/./work_ST0                     (1 files differ)
ww3_ts4/./work_ug_MPI                     (1 files differ)

aronroland and others added 22 commits November 5, 2021 16:43

scalability_reintegration: rmeove xyb since it is obsolete part 1

a2c7593

add missing parmetis dependencies in link.ite and add comp and link ite

c6c883d

(scalability_reintegration: rmeove xyb since it is obsolete part 2

85c172c

)

scalability_reintegration: rmeove xyb since it is obsolete part 3

65b5d62

scalability_reintegration: rmeove xyb since it is obsolete part 4

7e42a73

scalability_reintegration: rmeove xyb since it is obsolete part 5

2ae3725

Merge branch 'NOAA-EMC:develop' into scal_xyb

55aea11

(scalability_reintegration: remove xyb since it is obsolete part 6)

8640709

Merge branch 'scal_xyb' of https://github.com/erdc/WW3 into scal_xyb

80bb171

scalability_reintegration: remove xyb since it is obsolete part 7

7a5aa38

Merge branch 'NOAA-EMC:develop' into scal_xyb

0a2bfdd

remove xyb since it is obsolete part 8 - fix bug in ww3_outf

227bf5e

scalability_reintegration: remove xyb since it is obsolete part 8

e333028

Merge branch 'scal_xyb' of https://github.com/erdc/WW3 into scal_xyb

9d004ca

scalability_reintegration: remove xyb since it is obsolete part 9

8e6705e

scalability_reintegration: remove xyb since it is obsolete part 10

7ea5864

scalability_reintegration: remove xyb since it is obsolete part 11

c9a51e8

scalability_reintegration: remove xyb since it is obsolete part 11

e649518

scalability_reintegration: remove xyb since it is obsolete part 11 - …

ce38f99

…following the quest

scalability_reintegration: remove xyb since it is obsolete part 12

a034343

scalability_reintegration: remove xyb since it is obsolete part 12

8755eb1

resolve conflict

3d8b271

JessicaMeixner-NOAA requested changes Dec 7, 2021

View reviewed changes

aliabdolali and others added 3 commits December 9, 2021 19:14

rm comp.ite link.ite

0a9411e

rm comments

ffca813

Merge branch 'NOAA-EMC:develop' into scal_xyb

71a6106

aliabdolali and others added 6 commits December 21, 2021 08:39

Merge branch 'NOAA-EMC:develop' into scal_xyb

4dffd10

Merge branch 'NOAA-EMC:develop' into scal_xyb

6146991

scal_xyb: consolidate PR based on comments of Jessica

f8ac27a

scal_xyb: PR consolidation 2nd part

58c92fa

Merge branch 'scal_xyb' of https://github.com/erdc/WW3 into scal_xyb

db7b667

fix for extra ENDIF

83ce92a

aliabdolali requested a review from JessicaMeixner-NOAA December 24, 2021 12:40

JessicaMeixner-NOAA approved these changes Dec 30, 2021

View reviewed changes

aliabdolali merged commit 7e006d9 into NOAA-EMC:develop Dec 30, 2021

thesser1 deleted the scal_xyb branch October 26, 2023 14:09

Conversation

aronroland commented Dec 7, 2021 • edited by aliabdolali Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Summary

Description

Issue(s) addressed

Commit Message

Check list

Testing

Uh oh!

JessicaMeixner-NOAA commented Dec 7, 2021

Uh oh!

JessicaMeixner-NOAA left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JessicaMeixner-NOAA Dec 7, 2021

Choose a reason for hiding this comment

Uh oh!

aronroland Dec 23, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JessicaMeixner-NOAA commented Dec 14, 2021

Uh oh!

aliabdolali commented Dec 15, 2021

Uh oh!

JessicaMeixner-NOAA commented Dec 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aronroland commented Dec 15, 2021 via email

Uh oh!

JessicaMeixner-NOAA commented Dec 15, 2021

Uh oh!

aronroland commented Dec 15, 2021 via email

Uh oh!

JessicaMeixner-NOAA commented Dec 16, 2021

Uh oh!

aronroland commented Dec 17, 2021 via email

Uh oh!

ukmo-ccbunney commented Dec 17, 2021

Uh oh!

aronroland commented Dec 17, 2021

Uh oh!

JessicaMeixner-NOAA commented Dec 17, 2021

Uh oh!

aronroland commented Dec 18, 2021 via email

Uh oh!

JessicaMeixner-NOAA commented Dec 18, 2021

Uh oh!

aliabdolali commented Dec 21, 2021

Uh oh!

aliabdolali commented Dec 24, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aronroland commented Dec 7, 2021 •

edited by aliabdolali

Loading

JessicaMeixner-NOAA commented Dec 15, 2021 •

edited

Loading